-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix: slugFormatter populates date template variables using date from entry if it exists #7633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: slugFormatter populates date template variables using date from entry if it exists #7633
Conversation
|
@fionaRowan this is a very well-written PR! I like the idea of your second point: allow the user to configure which date to use. For instance, Hugo also uses But, how should the user pass that field? option 1:
The original proposal from #6801. It feels too verbose, and option 2:
This matches your option 3, I like it better. If I understand correctly, this is possible if we merge #6690? In any case, I would first merge what you did here, because it's a well-rounded PR. Just let me know once you complete the testing, and please share your thoughts on how to continue. |
01ba64f to
e0128c5
Compare
|
thanks for the thorough review, @martinjagodic ! to respond to some of your feedback and questions below~
I believe inferring the date field takes into account "synonyms", so this change should result in picking the first of those date “synonyms” - I added a couple tests in my last commit to be sure. that said, if we'd prefer not to infer which date field to use in this case, I could see value in introducing a new config variable, similar to
I agree with your take here! thanks for the insight!
I also like this option overall better, and overall I think there'd be value in merging #6690 as is to start supporting filtering in slug configs. some small caveats worth mentioning based on my own minimal unit testing:
lastly, it appears date handling varies depending on replacement method in
the difference between the above two examples appears to be a difference in string serialization~
there could be value in tracking the diverging behavior in date handling in a small follow-up issue - would be happy to file one if you agree these should be functionally equivalent examples! |
|
(I still plan to manually test my change later today, but happy to go for another implementation here or close this in favor of #6690 ! will defer to your insight here. thanks again for the review!) |
|
@martinjagodic finally wrapped up my manual testing~ below are the steps I took:
resulting file nameon this branch: on main: |

related to issue #6801 , though takes a different approach than that requested in the issue description. this PR tentatively implements solution 1 outlined in this issue comment, though could pivot to another solution if preferred!
Summary
currently, when configuring the slug, the template tags
{{year}},{{month}}, etc are always pulled from the hardcoded "now".in this PR, if the entry contains an inferred
'date'field, we use that value when generating the slug.thus, the following slug config would pull the template tags
{{year}},{{month}}, etc from the entry's front matter inferred'date'field if it exists (or otherwise, "now"):notes for evaluating slugFormatter's expected behavior w.r.t. date template tags:
slugFormatter's behavior consistent with that offolderFormatter, which already uses the date from the entry's front matter by default.slugFormatter's behavior with this piece of documented behavior:slugFormatter's behavior more nuanced than this piece of documented behavior impliesTest plan
(still working on this)built the library + pointed my own static generator site to that library locally to verify that the slug template variables{{year}},{{month}}, etc are pulled from the entry's front matter "date" field if it existsChecklist
Please add a
xinside each checkbox:A picture of a cute animal (not mandatory but encouraged)
